Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gd32vw55x flash driver #12

Merged
merged 3 commits into from
Feb 3, 2024

Conversation

JackChenHR
Copy link

gigadevice risc-v mcu

Copy link
Member

@fanghuaqi fanghuaqi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Alan-19950616
Copy link

@JackChenHR

Hi, Jack thanks for your work on OpenOCD.

Here are some coding formatting some issues, please refer to the attachment for fixes.

gd32vw55x_check.txt

}

# Allow overriding the Flash bank size
if { [info exists FLASH_SIZE] } {
Copy link

@Alan-19950616 Alan-19950616 Jan 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see any use for _FLASH_SIZE, line 32 has flash_size set to 0x400000.



# Work-area is a space in RAM used for flash programming
if { [info exists WORKAREASIZE] } {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see any use for _WORKAREASIZE , line 11 has work-area-size set to 20480.

@fanghuaqi
Copy link
Member

Hi @JackChenHR , could you do some update to your code reviewed by @Alan-19950616

@fanghuaqi
Copy link
Member

@JackChenHR

Hi, Jack thanks for your work on OpenOCD.

Here are some coding formatting some issues, please refer to the attachment for fixes.

gd32vw55x_check.txt

Hi @Alan-19950616 you can provide some steps regarding how to check styles locally to @JackChenHR

@JackChenHR
Copy link
Author

@Alan-19950616 Yes, please help provide a local method to check the code format. thanks!

@fanghuaqi
Copy link
Member

Hi @JackChenHR , I checked https://openocd.org/doc-release/doxygen/patchguide.html#stepbystep this page, you can take a try.

image

@Alan-19950616 Please provide a step by step example later to check the patch style, not just the link I provided.

@fanghuaqi
Copy link
Member

Hi @JackChenHR, here is the script you can check your commit patch

You need to install patchutils perl

# since you have commit two commits, this diff is  HEAD~2 , if you have more commits, such as 4, change to HEAD~4
git diff -U20 HEAD~2 | filterdiff -x "a/src/jtag/drivers/libjaylink/*" -x "a/tools/git2cl/*" -x "a/.github/*" | ./tools/scripts/checkpatch.pl --no-signoff -
# then you will be able to see style check output in command window

image

@fanghuaqi fanghuaqi changed the base branch from nuclei/2023.10 to nuclei/2023 January 25, 2024 11:01
@fanghuaqi fanghuaqi changed the base branch from nuclei/2023 to nuclei/2023.10 January 25, 2024 11:01
Signed-off-by: JackChenHR <[email protected]>
@fanghuaqi fanghuaqi merged commit f042794 into riscv-mcu:nuclei/2023.10 Feb 3, 2024
4 checks passed
@fanghuaqi
Copy link
Member

Hi @JackChenHR , I have merged the changes into our branch, thanks for your contribution, could you take a try with the nuclei/2023.10 branch, and see whether it works as expected to debug your device.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants